Fix Windows binary utility discovery on Arm64 - #2586

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

/ok to test daf96fb

PR 2586 second review: findings and recommendations (gpt-5.6.-sol)

Review target:

• PR: NVIDIA/cuda-python#2586
• Contributor branch: isVoid/agent/fix-woa-binary-utility-layouts
• Reviewed head: d063eb3
• Comparison base used for this review: 4b13910
• Local worktree: /wrk/forked/pr2586_review

Line references in this report refer to the reviewed head above.

Outcome

The refreshed branch addresses the earlier launcher-alias, search-composition,
Compute Sanitizer, test-coverage, and search-order documentation feedback. Two
actionable defects remain:

1. High: Native Windows architecture selection is still dependent on the
   Python version and environment. It can select the x64 Nsight target on an
   Arm64 host under supported x64 CPython 3.10/3.11.
2. Low: A corrupt or stale Nsight registry registration is silently treated
   as "not installed."

There is also one low-priority documentation follow-up and one explicit
backward-compatibility decision that should be confirmed before merge.

Finding 1: High - native architecture detection is version-dependent

Location

• cuda_pathfinder/cuda/pathfinder/_utils/windows_arch.py:39
• cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py:113
• cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py:126
• cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py:186
• cuda_pathfinder/tests/test_search_steps.py:150
• cuda_pathfinder/pyproject.toml:9

Current behavior

windows_machine_arch() claims to return the native Windows machine
architecture, but its implementation is only:

raw_machine = platform.machine()

The result controls these standalone paths:

• Nsight Systems: target-windows-x64 versus target-windows-armv8
• Nsight Compute fallback: target/windows-desktop-win7-x64 versus
  target/windows-desktop-win10-t23x-a64

The public docstring explicitly promises that x64 Python on an Arm64 machine
selects the Arm64 target independently of Python. platform.machine() does not
provide that guarantee across every Python version supported by this package.

Why Michael's experiment is real but does not close the issue

Michael reported that platform.machine() returned arm64 from both of the
interpreters he tested. That result is credible. The original review wording
was too broad because it did not identify the Python-version split:

CPython 3.12 added a WMI query before the old environment-variable path. It
falls back to the environment variables if WMI is unavailable or fails. That
explains why a current native interpreter and a current x64 interpreter can
both report ARM64, while leaving the supported 3.10/3.11 case unresolved.

This is not merely theoretical:

• cuda-pathfinder declares requires-python = ">=3.10".
• The repository's wheel matrix explicitly includes Python 3.10 and 3.11.
• Microsoft documents that x64 emulation on Windows on Arm is transparent and
  that an emulated x64 application needs a host-aware API such as
  IsWow64Process2 to identify the Arm64 host reliably.
• An independent WoA diagnostic records sysconfig.get_platform() == "win-amd64" for both interpreters, with platform.machine() == "ARM64" on
  Python 3.12 but platform.machine() == "AMD64" on Python 3.11.

The current unit test at cuda_pathfinder/tests/test_search_steps.py:150 only
mocks the possible strings returned by platform.machine(). It verifies string
normalization, not native-host detection under emulation.

User impact

With x64 CPython 3.10/3.11 on an Arm64 Windows host, windows_machine_arch()
can return x64. The public finder then probes the x64 standalone Nsight target
instead of the Arm64 target. If only the native Arm64 utility is usable or
present, discovery returns None even though the installation is valid. This
is the compatibility scenario the PR is intended to fix, on Python versions
the package still supports.

Recommended fix

Use IsWow64Process2(GetCurrentProcess(), ..., pNativeMachine) and map the
returned native PE machine value:

• IMAGE_FILE_MACHINE_AMD64 (0x8664) -> x64
• IMAGE_FILE_MACHINE_ARM64 (0xAA64) -> arm64

The API's pNativeMachine output is specifically defined as the native host
architecture and is independent of the current process emulation mode.

Implementation details for Codex:

1. Keep the Windows API import lazy so importing cuda.pathfinder on Linux
   does not access ctypes.WinDLL.
2. Set argtypes and restype for GetCurrentProcess and
   IsWow64Process2.
3. Raise a contextual exception if the API call fails; do not silently select
   an architecture.
4. If pre-Windows-10 support is required, feature-detect
   IsWow64Process2. A platform.machine() fallback is reasonable only on
   Windows versions where x64-on-Arm emulation cannot occur. Document that
   compatibility branch explicitly.
5. Reuse WINDOWS_PE_MACHINE_BY_ARCH by constructing the reverse mapping;
   do not duplicate architecture constants at call sites.

Existing implementation that can be ported

Michael's earlier dangling commit already contains a focused ctypes helper and
unit tests:

git show c5033c6d7bb -- cuda_pathfinder/cuda/pathfinder/_utils/windows_arch.py
git show c5033c6d7bb -- cuda_pathfinder/tests/test_search_steps.py

Port the _windows_native_machine() and windows_machine_arch() approach from
that commit into the refreshed branch. Do not cherry-pick the whole commit:
its other finder changes predate and conflict with the current branch's alias,
layout, and coverage fixes.

Required tests

Replace the mocked platform.machine() normalization tests with tests of the
native PE machine contract:

• native PE machine 0x8664 returns x64;
• native PE machine 0xAA64 returns arm64 even when the interpreter is x64;
• an unsupported PE machine value raises a clear exception;
• a failing Windows API call raises a clear exception;
• if an old-Windows fallback is retained, test API absence separately from an
  API call failure;
• retain the public finder composition tests proving the returned native
  architecture selects the expected Nsight directory.

Every newly generated unit test must carry the repository's per-test
@pytest.mark.agent_authored(model="<actual-model>") marker.

Acceptance criteria

• windows_machine_arch() does not derive the native architecture from the
  Python interpreter architecture or ordinary processor environment variables
  on Windows versions that support x64-on-Arm emulation.
• x64 Python 3.10 and 3.11 on WoA select the Arm64 standalone Nsight target.
• x64 Windows hosts continue to select x64.
• unsupported architectures and API failures are explicit errors.

Primary and corroborating references

• CPython 3.10 Windows machine implementation
• CPython 3.11 Windows machine implementation
• CPython 3.12 WMI-first implementation
• Microsoft: detecting Windows on Arm emulation
• Microsoft: IsWow64Process2
• PyInstaller's independently verified x64-Python-on-Arm handling
• Independent Python 3.12 versus 3.11 WoA diagnostic

Finding 2: Low - corrupt registry data is silently treated as not installed

Location

• cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py:86
• cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py:93
• cuda_pathfinder/tests/test_find_nvidia_binaries.py:419

Current behavior

_windows_installed_nsight_root() wraps all of these operations in one
except FileNotFoundError: return None block:

1. opening the product key;
2. reading CurrentVersion;
3. opening the selected version subkey;
4. reading the default installation-root value.

An absent outer product key is the normal "product is not installed" case.
Once that key exists, a missing CurrentVersion, missing version subkey, or
missing installation value indicates a corrupt or stale registration. Silently
returning None masks the reason discovery failed and conflicts with the
package guidance to keep discovery deterministic and explicit.

The existing test covers only a valid registration.

Recommended fix

Narrow the normal-miss handling to the initial product-key open:

try:
    product_context = winreg.OpenKey(...)
except FileNotFoundError:
    return None

with product_context as product_key:
    try:
        # Read CurrentVersion, open its subkey, and read the install root.
        ...
    except FileNotFoundError as exc:
        raise RuntimeError(
            f"Incomplete Nsight {product!r} registry registration at {product_key_path!r}"
        ) from exc

Keep the existing type/empty-value validation. Let unrelated errors such as
access denial propagate rather than converting them into a successful miss.

Add tests for:

• absent outer product key -> None;
• missing CurrentVersion -> contextual RuntimeError;
• invalid/empty CurrentVersion -> contextual RuntimeError;
• missing version subkey -> contextual RuntimeError;
• missing default installation value -> contextual RuntimeError;
• invalid/empty installation value -> contextual RuntimeError;
• valid entry -> installation root.

Also update the public find_nvidia_binary_utility() Raises section at
cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py:182.
It currently lists only UnsupportedBinaryError, although the new registry and
native-architecture paths can raise runtime/system errors. Prefer wrapping the
Windows API failure in a contextual RuntimeError and documenting that public
failure mode.

Documentation follow-up: Low - supported-platform text omits Windows Arm64

Location

• cuda_pathfinder/docs/source/install.rst:12
• cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py:186

The installation guide says the supported Windows platform is only x86-64,
while the PR now documents and implements behavior for native Arm64 Python and
x64 Python running on an Arm64 Windows host.

If native Windows Arm64 is an officially supported runtime after this PR,
change the requirement line to include Windows (x86-64, arm64). If the PR is
only intended to support x64 Python under WoA while native Arm64 Python remains
unsupported, state that narrower scope consistently instead. The current two
documents disagree.

Compatibility decision - terminal registry misses for nsys and ncu

This is not presented as a confirmed defect because the refreshed PR now makes
the behavior explicit and tests it. It is nevertheless a public behavior
change that should be consciously accepted.

Evidence

• Before the PR, every supported utility name could fall through from
  wheel/Conda lookup to CUDA_HOME/CUDA_PATH and then the canary-derived CTK
  root.
• At head, canonical Windows nsys and ncu return directly from the registry
  finder at
  cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py:273.
• The docstring says the registry miss is terminal at
  cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py:213.
• cuda_pathfinder/tests/test_find_nvidia_binaries.py:342 asserts that neither
  get_cuda_path_or_home() nor the canary is called after a registry miss.

The PR's standard standalone layouts place Nsight outside the CTK root, which
supports the new design. The compatibility risk is limited to older or
nonstandard installations where callers relied on an explicitly populated
CUDA_HOME/CUDA_PATH or canary-derived root containing nsys/ncu.

Recommendation

Confirm one of these choices in the PR:

1. Preserve compatibility: try the registry first, but continue to the
   existing CUDA-path and canary stages when the registry finder returns
   None. Replace the terminal-miss test with explicit-path and canary
   fallback tests. For ncu, consider probing ncu.bat before ncu.exe in
   those fallback directories as well.
2. Accept the intentional break: leave the implementation and regression
   test as-is, and call the behavior change out in release notes so users with
   nonstandard layouts have an actionable migration path.

Do not make this change mechanically without the maintainer's compatibility
decision.

Earlier findings that are now resolved

The second review confirmed these fixes at d063eb3cb22:

• Literal nsight-sys and nsight-compute lookup is preserved. These names
  are no longer remapped to the distinct nsys and ncu CLIs.
• Wheel and Conda candidates retain priority over standalone registry lookup.
• Public-call tests compose wheel/Conda misses, registry product selection,
  native target selection, and final path resolution.
• ncu.bat is preferred, with an architecture-specific direct ncu.exe
  fallback.
• Compute Sanitizer prefers bin/compute-sanitizer.bat, falls back to the
  nested executable, and now covers the canary-derived CTK root.
• Search-order prose matches the implemented terminal-control flow.
• Dynamic-library, static-library, and executable architecture contracts are
  documented separately.
• All newly added tests observed in the refreshed diff have per-test
  @pytest.mark.agent_authored(model="gpt-5.6") markers.

Verification status

• Worktree was clean at reviewed head d063eb3cb22.
• git diff --check 4b13910beb6..d063eb3cb22 passed.
• The PR description reports
  pixi run --manifest-path cuda_pathfinder -e cu13 test with 1305 passed,
  4 skipped, plus passing docs, scoped pre-commit, Ruff, and mypy checks.
• At the time of re-review, all listed functional checks passed except the
  aggregate check caused by one Windows H100x2 workflow cancellation at its
  60-minute limit. Its cuda.pathfinder test step passed before cancellation
  later during cuda.core.
• Windows PR CI currently uses ARCH: amd64; there is no WoA runner covering
  the high-severity architecture scenario.
• /wrk/forked/pr2586_review has no top-level TestVenv, so this report-writing
  pass did not invoke raw pytest or rerun tests outside the repository-managed
  NVIDIA workflow.

Suggested implementation order for Codex

1. Port the native-machine helper and focused tests from c5033c6d7bb.
2. Narrow registry FileNotFoundError handling and add corrupt-registration
   tests.
3. Update the public Raises contract and resolve the Windows Arm64 support
   wording in install.rst.
4. Ask the maintainer to choose whether canonical nsys/ncu registry misses
   remain terminal or fall through for backward compatibility.
5. Run the repository-managed package suite, mypy/Ruff, docs build, and
   pre-commit. On this NVIDIA host, use a top-level TestVenv if one is
   provided; otherwise ask how the test environment should be built rather
   than invoking raw pytest.